Skip to content

[MSHADE-124] keep original basedir after set d-r-p location to build dir - #128

Open
gzm55 wants to merge 2 commits into
apache:masterfrom
gzm55:work/MSHADE-124
Open

[MSHADE-124] keep original basedir after set d-r-p location to build dir#128
gzm55 wants to merge 2 commits into
apache:masterfrom
gzm55:work/MSHADE-124

Conversation

@gzm55

@gzm55 gzm55 commented Apr 5, 2022

Copy link
Copy Markdown

From maven 3.2.5, the pom and basedir are seperated
(apache/maven@c15226f),
and add new api MavenProject::setPomFile() to only upodate pom location.

In this pr, the minimal maven version is bumped to 3.2.5, then use
setPomFile to only update pom file location, leaving ${basedir}
unchanged.

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [MSHADE-XXX] - Fixes bug in ApproximateQuantiles,
    where you replace MSHADE-XXX with the appropriate JIRA issue. Best practice
    is to use the JIRA issue title in the pull request title and in the first line of the
    commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean verify to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the integration tests successfully (mvn -Prun-its clean verify).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@jira-importer

Copy link
Copy Markdown

Resolve #496

1 similar comment
@jira-importer

Copy link
Copy Markdown

Resolve #496

@altaiezior

Copy link
Copy Markdown

@cstamas I wanted to refer my previous comment for this MR, I wanted to change the base location of dependency reduced pom. Can we check this MR and resolves issues if any.

So my use-case here for this fix is to move the dependency-reduced-pom.xml to the target folder, the reason for this that I don't want my search results to include the values from this file.

Target folder is excluded by default in IntelliJ and hence this helps

@altaiezior

Copy link
Copy Markdown

@rmannibucau we can just add project.setOriginalModel(model); in this MR after project.setPomFile(dependencyReducedPomLocation); and it should be ready as well and work with maven4

@altaiezior

Copy link
Copy Markdown

@cstamas would it be possible to take this up?

@cstamas

cstamas commented Mar 16, 2026

Copy link
Copy Markdown
Member

@gzm55 care to rebase? Thanks!

@altaiezior

Copy link
Copy Markdown

Yes I can but I think I will have to create the MR again, since it is from a fork right? So the history would be lost?

@gzm55

gzm55 commented Mar 17, 2026

Copy link
Copy Markdown
Author

@gzm55 care to rebase? Thanks!

pr is rebased.

@altaiezior

Copy link
Copy Markdown

@cstamas can we check this now?

@altaiezior

altaiezior commented Mar 26, 2026

Copy link
Copy Markdown

@cstamas would you be able to review and merge this change sometime this week?

@altaiezior

Copy link
Copy Markdown

Hi @cstamas, just a gentle follow-up on this when you have a moment. Would really appreciate a review whenever you get the chance 🙂

@altaiezior

Copy link
Copy Markdown

@cstamas just following up on this PR, if there is anything we can do to get this merged?

@cstamas
cstamas requested a review from slawekjaranowski June 10, 2026 18:27
gzm55 and others added 2 commits June 12, 2026 19:27
From maven 3.2.5, the pom and basedir are seperated
(apache/maven@c15226f),
and add new api `MavenProject::setPomFile()` to only upodate pom location.

In this pr, the minimal maven version is bumped to 3.2.5, then use
`setPomFile` to only update pom file location, leaving `${basedir}`
unchanged.
@altaiezior

Copy link
Copy Markdown

@cstamas @slawekjaranowski any updates on when this can be released?

@slachiewicz

Copy link
Copy Markdown
Member

The fix looks right. setPomFile assigns the same file field as setFile and only skips the basedir reassignment, so getFile() still returns the reduced POM and install and deploy keep publishing it — the change drops the side effect without changing what ships. It carries @since 3.2.4, and this plugin requires 3.6.3, so the floor is clear.

One thing on the new IT guard. The lookbehind tests for /target, and a Windows basedir ends in \target, so the rule passes there whether or not the regression is present:

.*(?<!/target)$    against  C:\proj\target  ->  matches, rule passes
.*(?<![/\\]target)$ against C:\proj\target  ->  no match, rule fails

The shared workflow's default os-matrix includes windows-latest, so a third of the matrix runs the IT without the assertion doing anything. Accepting either separator restores it.

This comment was created with AI assistance.

@slachiewicz slachiewicz added the bug Something isn't working label Aug 24, 2026
@slachiewicz

Copy link
Copy Markdown
Member

For whoever picks this up: #827 and #828 propose a different fix for the same problem — saving project.getFile() before the dependency-reduced POM is written and restoring it afterwards, rather than this PR's switch from setFile to setPomFile. They are alternatives, not complements, so one of the three should be chosen before any is merged.

Both of those are currently red, on the Windows cell only, for an unrelated path assumption in their own ITs — the same /-only shape as the basedir regex noted above.

This comment was created with AI assistance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working waiting-for-feedback Waiting for 90 days until issues or pull request will be closed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants